fix(ios): loop animations leak the view via CAAnimation delegate retain cycle#50
Merged
Merged
Conversation
… loop snapshots CAAnimation strongly retains its delegate, and the loop animation snapshots saved in _loopAnimations never complete, so using the view as the animation delegate created a permanent retain cycle (view -> _loopAnimations -> animation -> view). Views released without going through prepareForRecycle (recycle pool at capacity, recycling disabled, surface teardown) leaked. Animations now use a proxy delegate that holds the view weakly and forwards animationDidStop, so completion events and the pending-animation bookkeeping behave as before. The all-'none' transition path also removed the layer animations without clearing _loopAnimations, so a cancelled loop resurrected the next time the view re-attached to a window. Clear the saved snapshots there too. Adds an example-app reproducer under Issues that cancels a loop and switches tabs to exercise the didMoveToWindow re-apply path.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
EaseViews with an infinite loop animation leaked on any release path that skips view recycling (recycle pool at capacity, recycling disabled, surface teardown). CAAnimation retains its delegate, we set
animation.delegate = self, and looping animations are also kept in_loopAnimationsfor re-adding on window re-attach — since an infinite loop never finishes, that's a permanent view →_loopAnimations→ animation → view cycle that onlyprepareForRecyclebroke. Found in a memory audit.Two changes:
animationDidStop:finished:(a proxy rather than nil-ing the delegate on the stored snapshot, soreapplyLoopAnimations'_pendingAnimationCountbookkeeping andonTransitionEndevents keep working).removeAllAnimationswithout clearing_loopAnimations, so a cancelled loop came back on the nextdidMoveToWindow. It now clears the saved snapshots too.Adds an example-app reproducer (Issues → "Audit — Cancelled loop resurrects") that cancels a loop via
transition={{ type: 'none' }}and uses tabs to exercise the re-attach path.Test Plan
Verified on the iOS simulator, first against a broken-baseline build (fixes reverted, dealloc logging,
shouldBeRecycled = NOto bypass the recycle pool), then the fixed build:reapplyLoopAnimations), and the Interrupt demo still reports bothonTransitionEndvalues through the proxy — "Finished" on completion, "Interrupted!" when a second toggle lands mid-animation (600 ms double-tap), then "Finished" again when the replacement animation completes.